Deduplicate tag ingredient EmiStack lists#933
Open
embeddedt wants to merge 1 commit intoemilyploszaj:1.21from
Open
Deduplicate tag ingredient EmiStack lists#933embeddedt wants to merge 1 commit intoemilyploszaj:1.21from
embeddedt wants to merge 1 commit intoemilyploszaj:1.21from
Conversation
Owner
|
Seems like a reasonable performance target, hashing the lists seems perhaps a bit wasteful though, surely ingredients could do lookups via tag key and then do comparison, or perhaps ignore all ingredients with mutated (chanced, remainder) elements |
embeddedt
force-pushed
the
tag-dedup
branch
2 times, most recently
from
e7fdb00 to
2b42705
Compare
Contributor
Author
|
I tried this and it's unfortunately a weaker optimization. Hashing the lists is more powerful as it can correctly deduplicate cases where many tag ingredients have mutated the ingredients in the exact same way (e.g. in Monifactory, there are a bunch of wrench tag ingredients where all the inner stacks have remainders with the wrench damaged by one more unit). The lookup approach barely saves anything in Monifactory, by comparison. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This will likely need some refinement before being merged; opening it now to collect feedback.
This PR implements deduplication of the underlying stack lists used by
TagEmiIngredientafter all recipes are constructed. Initial testing suggests it saves about 80MB of memory in Craftoria; gains should scale with packs containing larger tags that are used often. TheTagEmiIngredientobjects themselves are still duplicated, but their shallow size isn't the majority of memory usage from what I see.